fix(workspace): a corrupt container is not an empty workspace, and a hibernated terminal's tmux dies with it - #1048
Merged
Conversation
…hibernated terminal's tmux dies with it Three of the four follow-ups filed as #1030 from #1013's persistence review. Each is a fail-first test against the RECORDED live workspace (testing/fixtures/workspace-v2), degraded in exactly one field. 1. Item 3 — data loss. A `tabs` or `projects` field that is PRESENT but not a list migrated to an empty pool; rehydrate then minted a fresh tab and reported `complete`, which unlocks autosave, so the next 400 ms tick wrote an empty workspace over the real file. v2 threw here, and the throw is what puts bootstrap into its locked fallback with the disk file untouched. `undefined` stays legal: a v2 file has no `projects`, a v3 file has no `tabs`, and `tabs: []` is a writer's empty workspace rather than corruption. 2. Item 2 — data loss. A v2 file with zero tabs still carried its buried panes, because burial was independent of tabs there. Here every session needs a project, so with none left the re-parent target was '' and every buried row was dropped — including rows whose only copy of the session metadata was the buried record itself. The migration now mints one project to receive them, titled from the first such session's cwd. 3. Item 4 — resource leak. Closing a hibernated terminal left its tmux session running until the next launch's sweep: boot hibernates every pane but the focused lane, so main has no row to tear down, and the renderer's persisted metadata is the only place the name survives. The close now carries `tmuxName`, and main kills it only when the registry minted the name (`ownsSessionName`), which is the same prefix filter `listManagedSessions` applies. Item 1 (carrying v2 bury notes) is left open on #1030: v3 has no parked- note surface, so where the text would be shown is a product decision rather than a migration fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…op the tmux kill Codex review of #1048: 1. P1. The burial recovery keyed on `restoredMeta`, which legacyMemberships sets ONLY when the metadata is missing from `sessions` — it exists to carry the copy a buried record holds. The common shape is the opposite: v2 buries a pane and leaves its `sessions` row in place, so exactly those rows were still dropped, which is the bug the clause exists to fix. Any parked membership now mints the project, and the metadata is resolved from either source. 2. P1. The hibernated-terminal tmux kill is REMOVED from this PR. A shared name prefix does not prove ownership: an unknown session id and an arbitrary cwd could authorize killing any `agentcode-…` name, including one minted by another install — and tmux targets match as patterns, so the review killed a foreign session with `kill-session -t agentcode-` on an isolated server. Its safe form needs a main-authoritative persisted ownership binding plus exact `=name` targeting, which is its own change; #1030 item 4 stays open with that design recorded. 3. P2. A migration throw escaped `adoptWorkspace`, and the callers only logged it — so main, which had already transferred session routing and recorded a pending bequest, never heard a refusal and those sessions stayed pinned to a window that would never display them. Adoption now refuses on a migration failure exactly as it does on unreadable JSON. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n it Round-2 review finding. The recovery mint fired for ANY parked membership, but a minted project only ever receives rows that re-parent into it — and only a membership whose own project is gone (`projectId: null`) does that. A membership naming a live v2 tab keeps that name, which is not a project id, so it is dropped further down no matter what we mint. The shape that exposed it is a hybrid file: `projects: []` beside stale v2 tabs and their sessions. Migration minted a project, then dropped every session, leaving an empty nameless project — and a file with one project no longer looks empty to bootstrap, so the user lost the first-run path as well. The new test builds that hybrid from the recorded live workspace and fails without the restriction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three of the four follow-ups filed as #1030, from #1013's persistence review. Every test degrades exactly one field of the recorded live workspace (
testing/fixtures/workspace-v2/2026-09-19-live-workspace.sanitized.json), and all four assertions fail on main.1. A corrupt container migrated to an empty workspace (item 3, data loss)
A
tabsorprojectsfield that is present but not a list migrated to an empty pool. Rehydrate then minted a fresh tab and reportedcomplete, which unlocks autosave — so the next 400 ms tick wrote an empty workspace over the real file. v2 threw here, and that throw is what puts bootstrap into its locked fallback with the disk file untouched.Still legal, deliberately:
undefined— a v2 file has noprojects, a v3 file has notabs;tabs: []— a writer's genuinely empty workspace.2. A v2 file with zero tabs dropped its buried rows (item 2, data loss)
Burial was independent of tabs in v2. Here every session needs a project, so with none left the re-parent target was
''and every buried row was dropped — including rows whose only copy of the session metadata was the buried record. The migration now mints one project to receive them, titled from the first such session's cwd.3. A hibernated terminal leaked its tmux session (item 4)
Boot hibernates every pane but the focused lane, so main holds no row for such a terminal: the close found nothing, returned false, and the shell ran until the next launch's reconciliation swept it. The close now carries
tmuxName, and main acts on it only when the registry minted that name (ownsSessionName— the same prefix filterlistManagedSessionsapplies), so a renderer cannot name someone else's tmux session.Not in this PR
Item 1 (carrying v2 bury notes) stays open on #1030: v3 has no parked-note surface, so where that text would be shown is a product decision, not a migration fix.
Verification
npx tsc -bclean; 137 tests across the workspace-shape, session-manager, persistence and storage suites. Fail-first confirmed for all four new assertions by reverting each source change in turn.🤖 Generated with Claude Code